Skip to content

[9.3](backport #51911) handle PreInit error case for runReady.Close#51931

Open
mergify[bot] wants to merge 1 commit into
9.3from
mergify/bp/9.3/pr-51911
Open

[9.3](backport #51911) handle PreInit error case for runReady.Close#51931
mergify[bot] wants to merge 1 commit into
9.3from
mergify/bp/9.3/pr-51911

Conversation

@mergify

@mergify mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Proposed commit message

WHAT

filebeat/beater: close runReady on Manager.PreInit failure

PR #51864 (commit 00068f7) reordered the defers in Filebeat.Run so that defer fb.runReady.Close() runs before the managerEarlyStop defer (correct LIFO ordering to avoid a 5-second wait when the manager's stop callback calls StopWithContext). However, both defers were placed after the b.Manager.PreInit() call. If PreInit returns an error, Run exits before either defer is registered, leaving runReady.ch permanently unclosed.

An unclosed runReady.ch means any subsequent call to StopWithContext (e.g. via a SIGINT-triggered Manager.Stop that invokes the beat stop callback) will wait the full 5-second timeout before proceeding, rather than returning immediately.

WHY

Restores the invariant from before #51864: runReady is closed regardless of how Run exits. The fix adds an explicit fb.runReady.Close() on the PreInit error return path, which is the one path the deferred close cannot reach. A new unit test TestRunClosesRunReadyOnPreInitFailure directly asserts the channel is closed after Run returns due to a PreInit error; confirmed to fail against the buggy code and pass with the fix.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

Disruptive User Impact

None. This is an internal shutdown-sequencing fix. The only observable change is that Elastic Agent-managed Filebeat instances where PreInit fails will no longer incur a spurious 5-second delay during shutdown.

How to test this PR locally

go test ./filebeat/beater/ -run "TestRunClosesRunReadyOnPreInitFailure|TestStopWaitsForRunReady" -v -count=1 -race

Related issues

Use cases

Screenshots

Logs

This is an automatic backport of pull request #51911 done by [Mergify](https://mergify.com).

@leehinman @mergify
handle PreInit error case for runReady.Close (#51911)
7e63404 
* handle PreInit error case for runReady.Close

* fix fmt issue

(cherry picked from commit 397822f)

# Conflicts:
#	filebeat/beater/stop_test.go
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Jul 14, 2026
@mergify mergify Bot requested a review from a team as a code owner July 14, 2026 14:53
@mergify mergify Bot added the backport label Jul 14, 2026
@mergify mergify Bot requested review from andrzej-stencel and khushijain21 and removed request for a team July 14, 2026 14:53
@mergify mergify Bot added the conflicts There is a conflict in the backported pull request label Jul 14, 2026
@mergify

mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 397822f has failed:

On branch mergify/bp/9.3/pr-51911
Your branch is up to date with 'origin/9.3'.

You are currently cherry-picking commit 397822f0b.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   filebeat/beater/filebeat.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   filebeat/beater/stop_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@infra-vault-gh-plugin-prod

infra-vault-gh-plugin-prod Bot commented Jul 14, 2026

Copy link
Copy Markdown

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 14, 2026
@mergify mergify Bot mentioned this pull request Jul 14, 2026
Merged
6 tasks
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

TL;DR

The Filebeat: Run check/update job fails during go vet because this backport adds fb.runReady.Close() at filebeat/beater/filebeat.go:290, but the 9.3 target branch does not define Filebeat.runReady (or its supporting shutdown synchronization). This is a backport dependency/conflict issue, not a transient Buildkite failure.

Remediation

  • Backport the prerequisite Filebeat run/shutdown synchronization change (af686c255e1f319b47524835f63b06d31365d47b, synchronize filebeat run and shutdown functions (#51800)) before this follow-up, or resolve the conflict by porting its complete runReady implementation and related shutdown changes.
  • Re-run make -C filebeat check update followed by make check-no-changes; the current failure is expected to persist until runReady is defined.
Investigation details

Root Cause

The failed commit 7e63404c0092b95706415795f9424990d42174a1 changes only filebeat/beater/filebeat.go to call fb.runReady.Close() and adds filebeat/beater/stop_test.go. The Buildkite compiler reports that *Filebeat has no runReady field or method. The PR history also records that cherry-picking the source commit conflicted because filebeat/beater/stop_test.go was deleted by the target branch, confirming that this is an incomplete backport context.

Evidence

  • Build: https://buildkite.com/elastic/beats/builds/49393
  • Job/step: Filebeat: Run check/update (Filebeat build 34477)
  • Key log excerpt: beater/filebeat.go:290:6: fb.runReady undefined (type *Filebeat has no field or method runReady)
  • Related prerequisite: commit af686c255e1f319b47524835f63b06d31365d47b (synchronize filebeat run and shutdown functions (#51800))
  • Existing PR comment: Mergify reports deleted by us: filebeat/beater/stop_test.go during the cherry-pick.

Verification

  • Not run locally; the prefetched Buildkite log provides a deterministic compile/vet failure.

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@mergify

mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b mergify/bp/9.3/pr-51911 upstream/mergify/bp/9.3/pr-51911
git merge upstream/9.3
git push upstream mergify/bp/9.3/pr-51911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khushijain21 khushijain21 Awaiting requested review from khushijain21 khushijain21 is a code owner automatically assigned from elastic/elastic-agent-data-plane
@andrzej-stencel andrzej-stencel Awaiting requested review from andrzej-stencel andrzej-stencel is a code owner automatically assigned from elastic/elastic-agent-data-plane

At least 1 approving review is required to merge this pull request.

Assignees

@leehinman leehinman

Labels

backport bugfix conflicts There is a conflict in the backported pull request skip-changelog Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@leehinman